Skip to content

Fix android failed build - #128

Merged
aik-jahoda merged 3 commits into
dotnet:mainfrom
aik-jahoda:jajahoda/fixAndroid
Apr 9, 2021
Merged

Fix android failed build#128
aik-jahoda merged 3 commits into
dotnet:mainfrom
aik-jahoda:jajahoda/fixAndroid

Conversation

@aik-jahoda

Copy link
Copy Markdown
Contributor

Add certificates needed for android build.
Follow up for dotnet/runtime#50230

cc @elinor-fung

@wfurt wfurt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (assuming the files work on Android)

@elinor-fung

elinor-fung commented Mar 30, 2021

Copy link
Copy Markdown
Member

It looks like these added certs are still using RC2 for encryption, which is not supported on Android. We need these .noRC2 files versions to not use RC2 encryption. Dumping info for one of the added certs under TestDataCertificates:

>  openssl pkcs12 -in ./TestDataCertificates/testclienteku.contoso.com.noRC2.pfx -info                      
...
PKCS7 Encrypted data: pbeWithSHA1And40BitRC2-CBC, Iteration 2048

The .noRC2 versions of all the certs were created in order to avoid using RC2 (they use TripleDES instead). Dumping info for one of the certs under TestData:

> openssl pkcs12 -in ./TestData/testclienteku.contoso.com.noRC2.pfx -info 
...
PKCS7 Encrypted data: pbeWithSHA1And3-KeyTripleDES-CBC, Iteration 2000

@aik-jahoda

Copy link
Copy Markdown
Contributor Author

I added new commit,
the info is:

PKCS7 Encrypted data: pbeWithSHA1And3-KeyTripleDES-CBC, Iteration 2048

@elinor-fung

Copy link
Copy Markdown
Member

I just realized that the versions of testselfsignedclienteku.contoso.com.pfx and testselfsignedservereku.contoso.com.pfx under the TestDataCertificates folder are also using RC2 now. The existing versions in the TestData folder were already only using TripleDES instead of RC2, which is why we didn't have to add noRC2 versions originally. Could you create noRC2 versions of those too? Sorry, I know this is obnoxious.

@wfurt

wfurt commented Apr 2, 2021

Copy link
Copy Markdown
Member

I'm wondering if we should make the noRC2 simply the default rather than creating separate files.
I had similar problem as some super new versions of openssl (maybe in combination with distro) disable very old ciphers - like rc2 so we may see more failures going forward.

@elinor-fung

Copy link
Copy Markdown
Member

cc @bartonjs for thoughts on noRC2/RC2 as the default for cert test data
I'd guess we'd hit the same problems / go with the same default for X509Certificates.TestData

@bartonjs

bartonjs commented Apr 2, 2021

Copy link
Copy Markdown
Member

thoughts on noRC2/RC2 as the default for cert test data

We probably want to keep some in X509Certificates, since that's how Windows XP generated PFXes (and everyone else copied that behavior), but outside X509 it's not important... they can all be 3DES so long as they open everywhere.

@aik-jahoda

Copy link
Copy Markdown
Contributor Author

I'll remove all *.noRC2.pfx and replace *.pfx with triple des.

@aik-jahoda

Copy link
Copy Markdown
Contributor Author

@elinor-fung can you please verify the current version fulfil your requirements?

@elinor-fung elinor-fung left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified that we can load these on Android. Thanks, @aik-jahoda!

@aik-jahoda
aik-jahoda merged commit 7b3780a into dotnet:main Apr 9, 2021
@aik-jahoda
aik-jahoda deleted the jajahoda/fixAndroid branch April 9, 2021 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants